fix: remove unused vscode-languageclient dependency - #1912
fix: remove unused vscode-languageclient dependency#1912dependabot[bot] wants to merge 4 commits into
Conversation
dependabot
Bot
added
dependencies
a26b7f7 to
e9eaafc
Compare
|
Dependabot (@dependabot) rebase |
833d405 to
423bda3
Compare
|
Dependabot (@dependabot) rebase |
Bumps [vscode-languageclient](https://github.com/Microsoft/vscode-languageserver-node/tree/HEAD/client) from 6.0.0-next.9 to 10.1.0. - [Release notes](https://github.com/Microsoft/vscode-languageserver-node/releases) - [Commits](https://github.com/Microsoft/vscode-languageserver-node/commits/release/client/10.1.0/client) --- updated-dependencies: - dependency-name: vscode-languageclient dependency-version: 10.1.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
423bda3 to
c57d886
Compare
Replace the removed private protocol converter import with the public LSP types API and align the VS Code engine requirement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Changyong Gong (chagong)
left a comment
Changyong Gong (chagong)
left a comment
There was a problem hiding this comment.
Approved: the vscode-languageclient 10 migration uses supported APIs, aligns the VS Code engine, adds regression coverage, and all checks pass.
There was a problem hiding this comment.
🟡 Changes recommended
The new asWorkspaceEdit implementation doesn’t correctly handle SnippetTextEdit entries under WorkspaceEdit.changes, which can lead to incorrect edit application when snippet edits are returned in that form.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the extension’s language-client dependency to vscode-languageclient@10.1.0 and replaces the previous protocolConverter-based workspace-edit conversion with an in-repo asWorkspaceEdit implementation to handle newer LSP workspace edit shapes (including snippet edits and change annotations).
Changes:
- Bump
vscode-languageclientto10.1.0and raise VS Code engine requirement to^1.91.0. - Implement a custom
asWorkspaceEditconverter (including support for annotated/resource changes and snippet edits indocumentChanges). - Add a test suite covering workspace edit conversion basics.
File summaries
| File | Description |
|---|---|
src/commands/generationCommands.ts |
Replaces vscode-languageclient protocol converter usage with a custom LSP WorkspaceEdit → VS Code WorkspaceEdit conversion function. |
test/suite/generationCommands.test.ts |
Adds unit tests for the new asWorkspaceEdit conversion behavior. |
package.json |
Bumps vscode-languageclient, adds vscode-languageserver-types, and updates the VS Code engine requirement. |
package-lock.json |
Updates lockfile to reflect the new dependency versions. |
Review details
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The private converter was the package’s only usage. Keep the focused protocol conversion, restore the VS Code 1.88 minimum, and strengthen snippet-edit coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Changyong Gong (chagong)
changed the title
Changyong Gong (chagong)
left a comment
Changyong Gong (chagong)
left a comment
There was a problem hiding this comment.
Approved: the unused direct language-client dependency is removed, the supported protocol conversion preserves the VS Code 1.88 minimum, and current-head CI passes.
Regenerate the removal from the base lockfile so unrelated transitive semver resolution stays unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new test file has a TypeScript type error (unknown accessed as SnippetTextEdit) that should be fixed to keep tsc compilation passing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
Changyong Gong (chagong)
left a comment
Changyong Gong (chagong)
left a comment
There was a problem hiding this comment.
Approved: the unused direct language-client dependency is removed, the minimal lockfile preserves unrelated resolutions, and all checks pass on the current head.
There was a problem hiding this comment.
🔵 Needs a closer look
The new metadata conversion currently assumes annotationId is always a string key, which can drop inline ChangeAnnotation objects accepted by the upstream type guards and lose edit metadata.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/commands/generationCommands.ts:32
AnnotatedTextEdit.is(...)andSnippetTextEdit.is(...)invscode-languageserver-typesacceptannotationIdvalues that are either a string identifier or an inlineChangeAnnotationobject. The currentmetadata()helper assumesannotationIdis always a string key intoprotocolEdit.changeAnnotations, so inline annotations would be silently dropped and metadata (label/needsConfirmation/description) lost.
Consider supporting both shapes by treating non-string annotationId values as the annotation itself, and also avoid coercing needsConfirmation to false when it’s omitted (preserve undefined).
- Files reviewed: 3/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
The original update exposed that
vscode-languageclientwas used only through a removed private protocol-converter import. The generation command now converts the returned LSP workspace edit through the publicvscode-languageserver-typesAPI, so the directvscode-languageclientdependency is removed instead of upgraded.This also preserves the existing VS Code 1.88 minimum and adds regression coverage for text edits, annotated resource changes, and snippet edits.